staticaddr: validate server address parameters#1137
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the security and robustness of the static address creation process by introducing server-side parameter validation. By ensuring that the parameters provided by the server conform to expected constraints before they are utilized in address generation, the system is better protected against malformed or malicious inputs. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces validation for server-provided parameters during the generation of new static addresses. A new validateServerAddressParams function ensures that the server response is not nil and that the expiry value is non-zero, within acceptable bounds, and contains valid CSV flags. Unit tests have been expanded to cover these validation scenarios. The review feedback recommends further strengthening the validation by explicitly checking for the presence of the server's public key and adding a corresponding test case.
hieblmi
force-pushed
the
param-validation
branch
2 times, most recently
from
df971c5 to
dcc6d58
Compare
| return fmt.Errorf("missing server address parameters") | ||
| } | ||
|
|
||
| if len(params.GetServerKey()) == 0 { |
There was a problem hiding this comment.
Can we validate that it is a serialized compresses EC public key?
https://pkg.go.dev/github.com/btcsuite/btcd/btcec/v2#IsCompressedPubKey
It checks both the length and the first byte.
There was a problem hiding this comment.
thanks for the improvement, I added it.
| return fmt.Errorf("invalid static address CSV flags: %x", | ||
| expiry) |
There was a problem hiding this comment.
Let's rephrase it to be more clear:
"static address expiry does not fit into CSV: %x"
| } | ||
| } | ||
|
|
||
| func newServerNewAddressResponse(expiry uint32) *swapserverrpc.ServerNewAddressResponse { |
3 participants
This PR adds validation of server address parameters during the creation of static addresses.